Preserve skill upload file paths#1947
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6896b2bbf8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ('path' in value && value.path && String(value.path)))) || | ||
| ''; | ||
|
|
||
| if (options.preserveFilePaths) return name || undefined; |
There was a problem hiding this comment.
Avoid preserving full URLs as upload filenames
When preserveFilePaths is enabled for skill uploads, getName still takes name from a Response.url, so files: [await fetch('https://cdn.example.com/my-skill/SKILL.md')] (allowed by Uploadable) now sends the multipart filename as the full URL instead of a relative path or basename. That gives the skill service an invalid tree entry and can leak the remote host in the uploaded path; URL-derived names should be normalized to a pathname/basename rather than returned verbatim.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 243c17f. Response.url now falls back to the basename even when file paths are preserved, while File.name and stream paths still keep directory names.
Summary
skills.createregressionsFixes #1807
Testing
jest tests/form.test.ts --runInBandjest tests/api-resources/skills/skills.test.ts --runInBand -t "preserves directory paths"prettier --check src/internal/uploads.ts src/resources/skills/skills.ts src/resources/skills/versions/versions.ts tests/form.test.ts tests/api-resources/skills/skills.test.tsgit diff --check